chore(telemetry): add filter tracking for bulk ops COMPASS-10297#8015
chore(telemetry): add filter tracking for bulk ops COMPASS-10297#8015
Conversation
| async runBulkDelete() { | ||
| this.track('Bulk Delete Executed', {}, this.connectionInfoRef.current); | ||
| this.track( | ||
| 'Bulk Delete Executed', |
There was a problem hiding this comment.
I think both of these events should be called either after the operation succeeds or just before it (dataservice call). In delete event we show a confirmation modal, to which user can ignore and we still track it. Similarly for update, if the query is invalid, we still track it.
There was a problem hiding this comment.
Pull request overview
This PR adds telemetry to track whether a filter was present when running bulk update and bulk delete operations.
Changes:
- Extend
Bulk Update ExecutedandBulk Delete Executedtelemetry event payloads with ahas_filterboolean. - Populate the new
has_filterfield from the CRUD query bar state when executing bulk update/delete.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/compass-telemetry/src/telemetry-events.ts | Adds has_filter to bulk update/delete executed telemetry event payload definitions. |
| packages/compass-crud/src/stores/crud-store.ts | Sends has_filter in telemetry when bulk update/delete operations are executed. |
| { | ||
| isUpdatePreviewSupported: this.state.isUpdatePreviewSupported, | ||
| has_filter: !!this.queryBar.getLastAppliedQuery('crud').filter, |
There was a problem hiding this comment.
has_filter is computed using !!...filter, but getLastAppliedQuery() returns a query object where filter is always an object (often {}), which is truthy. This will report has_filter: true even when no filter is set. Compute it like the existing Query Executed telemetry does (e.g., Object.keys(filter ?? {}).length > 0) by first grabbing const { filter } = this.queryBar.getLastAppliedQuery('crud').
There was a problem hiding this comment.
Copilot is right here, filter can be {} which would be truthy. 'Query Executed' has the same pattern.
| name: 'Bulk Delete Executed'; | ||
| payload: Record<string, never>; | ||
| payload: { | ||
| /** Specifies if a filter was set in the query */ | ||
| has_filter: boolean; | ||
| }; |
There was a problem hiding this comment.
This event payload shape changed from {} to include has_filter. There are existing telemetry assertions in e2e tests that expect an empty payload / only isUpdatePreviewSupported (e.g., packages/compass-e2e-tests/tests/collection-bulk-delete.test.ts and collection-bulk-update.test.ts). These will fail until updated to assert the new property.
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes